Skip to content

ENH: Styler.apply(map)_index made compatible with Styler.to_excel #41995

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 54 commits into from
Oct 19, 2021

Conversation

attack68
Copy link
Contributor

@attack68 attack68 commented Jun 14, 2021

This is a follow-on to #41893.

It makes the new Styler.apply_index and Styler.applymap_index methods compatible with Styler.to_excel. If the methods are not used the output reverts to the old format.

Screen Shot 2021-06-14 at 10 58 47

MultiIndex Case

midx = pd.MultiIndex.from_product([['ix', 'jy'], [0, 1], ['x3', 'z4']])
df = pd.DataFrame([np.arange(8)], columns=midx)
def highlight_x(s):
    return ["background-color: yellow;" if 'x' in v else "" for v in s]
df.style.apply_index(highlight_x, axis="columns", level=[0, 2]).to_excel("mi_test.xlsx")

Screen Shot 2021-08-12 at 09 24 15

NOTE on TESTS

The tests for styler_to_excel were broken and were xfailed out of production in 2019. Since they dont work I have replaced them with some other, cleaner, tests and add tests relevant to this PR also.

Closes #25351

@attack68 attack68 changed the title Styler apply index specific excel [WIP] ENH: Styler.apply(map)_header made compatible with Styler.to_excel (for non-MultiIndex) Jun 14, 2021
attack68 added 2 commits June 20, 2021 20:34
…x_specific

# Conflicts:
#	doc/source/reference/style.rst
#	pandas/io/formats/style_render.py
#	pandas/tests/io/formats/style/test_html.py
@attack68 attack68 added Enhancement IO Excel read_excel, to_excel Styler conditional formatting using DataFrame.style labels Jun 29, 2021
@@ -472,6 +472,7 @@ def __init__(
self.na_rep = na_rep
if not isinstance(df, DataFrame):
self.styler = df
self.styler._compute()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this needed? init doesn't do anything really except for set things

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this clause is only called when you do styler.to_excel. It needs _compute to calculate the styles added. Previously, this was done in _generate_body below, but, since, it is now needed for both body and headers I brought it to the obvious point: initialisation, to avoid having to unnecessarily do it twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could maybe move this to styler.to_excel and maybe do the pre-calc there before passing to ExcelFormatter. Might be more sensible like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could maybe move this to styler.to_excel and maybe do the pre-calc there before passing to ExcelFormatter. Might be more sensible like that.

I tried it and it didn't work. The new tests failed when using a custom ExcelFormatter, the styles computation needs to be done within ExcelFormatter.

@attack68
Copy link
Contributor Author

attack68 commented Sep 25, 2021

I discovered #25351 where basically the tests for Styler.to_excel had all been removed, and were pretty ugly anyway.

So I have replaced these with a minimalist set of non-comprehensive tests, and also added the tests relevant to this PR.

@@ -582,7 +589,10 @@ def _format_header_mi(self) -> Iterable[ExcelCell]:
# Format in legacy format with dots to indicate levels.
for i, values in enumerate(zip(*level_strs)):
v = ".".join(map(pprint_thing, values))
yield ExcelCell(lnum, coloffset + i + 1, v, self.header_style)
if styles:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

worth making a function to do this conversion as this is repeated below

@@ -7,163 +7,158 @@
from pandas.io.excel import ExcelWriter
from pandas.io.formats.excel import ExcelFormatter

pytest.importorskip("jinja2")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is essentially a new requirement for this right? its ok ,but let's fully document this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, actually this file only had one test and that test already had importskip("jinja2"), so I haven't added anything, just moved it top level to share across all tests.

I noticed, however, that technically it is would be possible to create a Styler, apply methods and then do .to_excel all without jinja2. The only problem is that Styler does a check for jinja2 at initialisation, so this is not currently functionally possible.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its fine to require jinja2 for this, just mention this in the docs / doc-string somewhere. (pretty sure we should be testing this but pls ensure somewhere (and skipping elsewhere)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is an import error warning if jinja2 not present. But could be more explicit. Will do as a follow up.

@attack68 attack68 requested a review from jreback October 3, 2021 14:34
@@ -472,12 +493,14 @@ def __init__(
self.na_rep = na_rep
if not isinstance(df, DataFrame):
self.styler = df
self.styler._compute() # calculate applied styles
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this need to be called? e.g. shouldn't this be in render?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok i see your expl. i guess this is fine.

@@ -7,163 +7,158 @@
from pandas.io.excel import ExcelWriter
from pandas.io.formats.excel import ExcelFormatter

pytest.importorskip("jinja2")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its fine to require jinja2 for this, just mention this in the docs / doc-string somewhere. (pretty sure we should be testing this but pls ensure somewhere (and skipping elsewhere)

@attack68
Copy link
Contributor Author

attack68 commented Oct 5, 2021

@jreback ping green

@@ -472,12 +493,14 @@ def __init__(
self.na_rep = na_rep
if not isinstance(df, DataFrame):
self.styler = df
self.styler._compute() # calculate applied styles
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok i see your expl. i guess this is fine.

@jreback jreback added this to the 1.4 milestone Oct 19, 2021
@jreback jreback merged commit f2abbd9 into pandas-dev:master Oct 19, 2021
@jreback
Copy link
Contributor

jreback commented Oct 19, 2021

thanks @attack68

@attack68 attack68 deleted the styler_apply_index_specific_excel branch October 19, 2021 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement IO Excel read_excel, to_excel Styler conditional formatting using DataFrame.style
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ERR: excel styler tests failing recently
2 participants